Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Investigate Test Coverage Drop #427

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

0xNeshi
Copy link
Collaborator

@0xNeshi 0xNeshi commented Nov 29, 2024

Towards #426

Discussion

Below is a list of problems affecting our test coverage and possible mitigation steps:

  1. Solidity code used in sol! macros (events, errors, storage) is included in test coverage (effect: high).
    • Mitigation: group Solidity code in an internal module annotated with #[cfg_attr(coverage, coverage(off))] (see example, and pseudo code is below).
    #[cfg_attr(coverage, coverage(off))]
    mod sol_defs {
        sol! {
            event SomeEvent();
            // ...
        }
    
        sol! {
            error SomeError();
            // ...
        }
    
        // NOTE: has to be included, as `SolidityError` affects coverage,
        // so has to be ignored
        #[derive(SolidityError, Debug)]
        pub enum Error {
            // ...
        }
    
        sol_storage! {
            pub struct State {
                // ...
            }
        }
    }
    
    // use in the contract
    use sol_defs::*;
    // reexport the `State` struct to make visible outside the crate
    pub use sol_defs::State;
  2. Missing unit tests for functions that interact with other contracts (effect: high).
  3. Re-exported functions missing unit tests (effect: medium/high).
    • Mitigation 1: duplicate missing unit tests.
    • Mitigation 2: annotate relevant functions with #[cfg_attr(coverage, coverage(off))].
  4. Missing unit tests for certain flows, and even whole functions in certain contracts and in contracts/src/utils (effect: medium).
    • Mitigation: add missing unit tests.
  5. Using internal functions instead of public ones (e.g. IErc20::transfer is tested by calling _update, causing code coverage to register transfer as "unconvered") (effect: medium).
    • Mitigation: test publicly exposed fns, not the internal ones.
  6. IErc165-related tests exist verifying the generated INTERFACE_ID value is correct, but no tests exist that call <ContractName>::supports_interface (effect: low).
    • Mitigation: add missing supports_interface tests.
  7. MethodError impls uncovered (effect: low).
    • Mitigation: annotate with #[cfg_attr(coverage, coverage(off))].
  8. #[public] attr is included in coverage (effect: low).
    • Mitigation: open to suggestions.
  9. #[cfg(test)] mod tests (unit tests) are included in coverage (effect: none). NOTE: technically not a problem atm, as the effect of this is positive or neutral. If we still wanted to exclude them, see mitigation steps below.

Copy link

netlify bot commented Nov 29, 2024

Deploy Preview for contracts-stylus canceled.

Name Link
🔨 Latest commit f837803
🔍 Latest deploy log https://app.netlify.com/sites/contracts-stylus/deploys/6749ac8d3625d6000821fa3a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant